Skip to content

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Dec 31, 2021

This reduces the size of the __getit function for LOCAL_PANIC_COUNT and should speed up accesses of LOCAL_PANIC_COUNT a bit.

@bjorn3 bjorn3 added the A-thread-locals Area: Thread local storage (TLS) label Dec 31, 2021
@rust-highfive
Copy link
Contributor

r? @joshtriplett

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 31, 2021
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup=never (plausible perf effect, but should be positive or neutral)

@bors
Copy link
Collaborator

bors commented Dec 31, 2021

📌 Commit 6e4c3b1 has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 31, 2021
@bors
Copy link
Collaborator

bors commented Jan 1, 2022

⌛ Testing commit 6e4c3b1 with merge 64aaec50341510cde64227a2f67a73d8be26a35e...

@bors
Copy link
Collaborator

bors commented Jan 1, 2022

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 1, 2022
@rust-log-analyzer

This comment has been minimized.

@bjorn3
Copy link
Member Author

bjorn3 commented Jan 1, 2022

curl: (6) Could not resolve host: ci-mirrors.rust-lang.org

@bors retry spurious network error

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 1, 2022
@bors
Copy link
Collaborator

bors commented Jan 1, 2022

⌛ Testing commit 6e4c3b1 with merge 4484eacdad2cfbdfbfa56fbcb413dac1723889ff...

@bors
Copy link
Collaborator

bors commented Jan 1, 2022

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 1, 2022
@rust-log-analyzer

This comment has been minimized.

@bjorn3
Copy link
Member Author

bjorn3 commented Jan 1, 2022

Why does the unsafe_op_in_unsafe_fn lint trigger inside thread_local!{} here?

@bjorn3
Copy link
Member Author

bjorn3 commented Jan 10, 2022

Seems that const thread_local! was missing a couple of unsafe {} blocks.

@bjorn3
Copy link
Member Author

bjorn3 commented Jan 10, 2022

@rustbot ready

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 23, 2022
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 20, 2022
@bors
Copy link
Collaborator

bors commented Apr 2, 2022

☔ The latest upstream changes (presumably #95581) made this pull request unmergeable. Please resolve the merge conflicts.

@bjorn3 bjorn3 force-pushed the const_tls_local_panic_count branch from 8a0c633 to bfd45e0 Compare April 2, 2022 18:25
@bjorn3
Copy link
Member Author

bjorn3 commented Apr 2, 2022

@rust-lang/infra For some reason the x86_64-gnu-llvm-12, job got canceled. I tried rerunning it but then @github-actions canceled it again. Any idea what could be going on?

image

@bors
Copy link
Collaborator

bors commented Apr 13, 2022

☔ The latest upstream changes (presumably #95727) made this pull request unmergeable. Please resolve the merge conflicts.

This reduces the size of the __getit function for LOCAL_PANIC_COUNT and should
speed up accesses of LOCAL_PANIC_COUNT a bit.
@bjorn3 bjorn3 force-pushed the const_tls_local_panic_count branch from bfd45e0 to cbc0a15 Compare April 23, 2022 10:10
@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Apr 23, 2022
@bjorn3
Copy link
Member Author

bjorn3 commented Apr 23, 2022

Rebased

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 22, 2022
@JohnTitor
Copy link
Member

r? @Mark-Simulacrum as you reviewed previously

@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented May 23, 2022

📌 Commit cbc0a15 has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 23, 2022
@bors
Copy link
Collaborator

bors commented May 23, 2022

⌛ Testing commit cbc0a15 with merge ef9b498...

@bors
Copy link
Collaborator

bors commented May 23, 2022

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing ef9b498 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 23, 2022
@bors bors merged commit ef9b498 into master May 23, 2022
@bors bors deleted the const_tls_local_panic_count branch May 23, 2022 15:45
@rustbot rustbot added this to the 1.63.0 milestone May 23, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (ef9b498): comparison url.

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results
  • Primary benchmarks: 😿 relevant regression found
  • Secondary benchmarks: mixed results
Regressions 😿
(primary)
Regressions 😿
(secondary)
Improvements 🎉
(primary)
Improvements 🎉
(secondary)
All 😿 🎉
(primary)
count1 1 2 0 1 1
mean2 2.0% 1.3% N/A -1.5% 2.0%
max 2.0% 1.5% N/A -1.5% 2.0%

Cycles

Results
  • Primary benchmarks: 😿 relevant regression found
  • Secondary benchmarks: 😿 relevant regressions found
Regressions 😿
(primary)
Regressions 😿
(secondary)
Improvements 🎉
(primary)
Improvements 🎉
(secondary)
All 😿 🎉
(primary)
count1 1 3 0 0 1
mean2 1.9% 2.5% N/A N/A 1.9%
max 1.9% 2.8% N/A N/A 1.9%

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. number of relevant changes 2

  2. the arithmetic mean of the percent change 2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-thread-locals Area: Thread local storage (TLS) merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.